feat: multi-address DNS resolution for contact points and connections (DRIVER-201) - #890
feat: multi-address DNS resolution for contact points and connections (DRIVER-201)#890nikagra wants to merge 9 commits into
Conversation
…VER-201) newControlReconnectionQueryPlan() now creates copies of the original contact-point nodes (with their unresolved hostname endpoints) instead of synthetic nodes with resolved IPs. This ensures the control channel carries the hostname endpoint, which is preserved in metadata after topology refresh. DNS expansion for connection fallback is handled by ChannelFactory (PR scylladb#890), so the control-reconnection path does not need to inject resolved-IP nodes into the query plan. Also adds getContactPoints() stub back to LoadBalancingPolicyWrapperTest so tests that cover the control-reconnect path continue to pass.
Before-init query plan now uses getContactPoints() (original unresolved hostname nodes) instead of getResolvedContactPoints(). The DNS expansion to all IPs happens at the ChannelFactory level (PR scylladb#890), so expanding here was redundant and broke should_connect_with_mocked_hostname by replacing hostname endpoints with resolved-IP endpoints. Also remove the should_connect_when_first_dns_entry_is_non_responsive integration test from this PR; it belongs in PR scylladb#890 where ChannelFactory expansion actually enables it to pass.
There was a problem hiding this comment.
Pull request overview
Part 2/2 of DRIVER-201: extends the EndPoint API and ChannelFactory so that a hostname mapping to multiple IPs is tried address-by-address at the connection layer, instead of only the first IP. The EndPoint.resolve() method is deprecated in favor of a new resolveAll() default method; DefaultEndPoint, SniEndPoint, and ClientRoutesEndPoint override it; ChannelFactory.connect() now iterates over candidates and only fails when all are exhausted, while keeping protocol-version downgrade scoped to a single address.
Changes:
- Add
EndPoint.resolveAll()(default impl delegating to deprecatedresolve()); override inDefaultEndPoint,SniEndPoint,ClientRoutesEndPoint. - Rework
ChannelFactory.connect()intotryNextCandidate/connectToAddressso per-address failures fall back to the next IP while protocol-version downgrades stay scoped to one address. - Add unit tests for
DefaultEndPoint.resolveAll()and a newSniEndPointTest.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java | Deprecates resolve(); adds default resolveAll() method. |
| core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java | Overrides resolveAll() using InetAddress.getAllByName with single-address fallback. |
| core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java | Overrides resolveAll() returning one address per sorted A-record. |
| core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java | Overrides resolveAll() to wrap the single topology-monitor address. |
| core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java | Adds candidate-iteration and per-address protocol-negotiation methods. |
| core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java | New tests for resolveAll() (resolved, unresolved expansion, unresolvable fallback). |
| core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java | New test class covering SNI resolveAll() happy path, unresolvable host, and resolve() sanity check. |
Comments suppressed due to low confidence (1)
core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java:303
- When
connectToAddressfails withUnsupportedProtocolVersionException.forNegotiation(i.e. all protocol downgrades exhausted),tryNextCandidatewill treat this like any other per-address failure and try the next IP, even though the protocol-negotiation failure is a server-wide condition that will recur on every other IP of the same node. This also reuses the sharedattemptedVersionsCopyOnWriteArrayListacross candidates, so on each subsequent address the downgrade loop re-attempts the same protocol versions and adds duplicate entries, and the final exception ultimately reported will list each version multiple times. Consider distinguishing non-address-specific failures (UnsupportedProtocolVersionException, authentication errors, etc.) and short-circuiting the candidate loop in those cases.
perAddressFuture.whenComplete(
(channel, error) -> {
if (error == null) {
resultFuture.complete(channel);
} else if (index + 1 < candidates.length) {
LOG.debug(
"[{}] Failed to connect to {} ({}), trying next address",
logPrefix,
candidate,
error.getMessage());
tryNextCandidate(
endPoint,
shardingInfo,
shardId,
options,
nodeMetricUpdater,
currentVersion,
isNegotiating,
attemptedVersions,
resultFuture,
candidates,
index + 1);
} else {
// Note: might be completed already if the failure happened in initializer()
resultFuture.completeExceptionally(error);
}
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
05553f3 to
f631971
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Sequence Diagram(s)sequenceDiagram
participant ChannelFactory
participant EndPoint
participant tryNextCandidate
participant connectToAddress
participant resultFuture
ChannelFactory->>EndPoint: resolveAll()
EndPoint-->>ChannelFactory: SocketAddress[] candidates
ChannelFactory->>tryNextCandidate: attempt candidate at index 0
tryNextCandidate->>connectToAddress: connect using perAddressFuture
alt connection succeeds
connectToAddress-->>tryNextCandidate: DriverChannel
tryNextCandidate->>resultFuture: complete successfully
else connection or negotiation fails
connectToAddress-->>tryNextCandidate: complete perAddressFuture exceptionally
tryNextCandidate->>tryNextCandidate: attempt next candidate
end
tryNextCandidate->>resultFuture: fail after all candidates
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
f631971 to
860a34d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java`:
- Around line 222-242: The code calls endPoint.resolveAll() and passes the
resulting candidates array into tryNextCandidate() which immediately indexes
candidates[0]; guard against null or empty results by validating the output of
endPoint.resolveAll()—if it returns null or candidates.length == 0, complete
resultFuture exceptionally (or create a specific error) and return; otherwise
call tryNextCandidate(...) with the non-empty candidates. Update the block
around resolveAll(), candidates, and the call to tryNextCandidate() to perform
this check and fail fast via resultFuture.completeExceptionally when
appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ad3d5b5-6473-4c88-8777-93861f5de639
📒 Files selected for processing (12)
core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.javacore/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.javacore/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java
860a34d to
a6d0e48
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java (1)
37-37: ⚡ Quick winConsider adding test coverage for resolveAll() throwing an exception.
The
ChannelFactory.connect()implementation includes a catch block for exceptions thrown byresolveAll()(see context snippet 1, line 232). Adding a third test case where the mockedEndPoint.resolveAll()throws an exception (e.g.,UnknownHostException) would ensure all three defensive paths are tested:
- ✓ Returns null (covered)
- ✓ Returns empty array (covered)
- ✗ Throws exception (not covered)
📋 Suggested test case
`@Test` public void should_fail_future_when_resolve_all_throws_exception() { // Given when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); ChannelFactory factory = newChannelFactory(); EndPoint badEndPoint = mock(EndPoint.class); RuntimeException testException = new RuntimeException("DNS lookup failed"); when(badEndPoint.resolveAll()).thenThrow(testException); // When CompletionStage<DriverChannel> channelFuture = factory.connect( badEndPoint, null, null, DriverChannelOptions.DEFAULT, NoopNodeMetricUpdater.INSTANCE); // Then – future must complete exceptionally with the thrown exception assertThatStage(channelFuture) .isFailed(e -> assertThat(e).isSameAs(testException)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java` at line 37, Add a third test in ChannelFactoryResolveAllGuardTest that verifies ChannelFactory.connect() propagates exceptions thrown by EndPoint.resolveAll(): mock an EndPoint (e.g., badEndPoint) to throw a RuntimeException (or UnknownHostException) from resolveAll(), create the factory via newChannelFactory(), call factory.connect(badEndPoint, ...) with DriverChannelOptions.DEFAULT and NoopNodeMetricUpdater.INSTANCE, and assert the returned CompletionStage<DriverChannel> completes exceptionally with the same exception; this mirrors the existing tests for null/empty resolveAll() and targets the catch path in ChannelFactory.connect().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java`:
- Line 37: Add a third test in ChannelFactoryResolveAllGuardTest that verifies
ChannelFactory.connect() propagates exceptions thrown by EndPoint.resolveAll():
mock an EndPoint (e.g., badEndPoint) to throw a RuntimeException (or
UnknownHostException) from resolveAll(), create the factory via
newChannelFactory(), call factory.connect(badEndPoint, ...) with
DriverChannelOptions.DEFAULT and NoopNodeMetricUpdater.INSTANCE, and assert the
returned CompletionStage<DriverChannel> completes exceptionally with the same
exception; this mirrors the existing tests for null/empty resolveAll() and
targets the catch path in ChannelFactory.connect().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b702fd48-9ba7-4994-8bb9-351438fb02a8
📒 Files selected for processing (13)
core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.javacore/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.javacore/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.javacore/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java
✅ Files skipped from review due to trivial changes (5)
- core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
- core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java
- core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java
- core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java
🚧 Files skipped from review as they are similar to previous changes (7)
- core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
- core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java
- core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java
- core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java
a6d0e48 to
f9265b3
Compare
|
🤖: Valid nitpick. Added a third test |
f9265b3 to
4448119
Compare
4448119 to
1c8dfa2
Compare
|
Rebased this PR (Part 2/2) on top of #889 ( Also addressed the outstanding review feedback:
Previously-addressed items (Copilot / CodeRabbit) remain in place after the rebase: N×timeout Javadoc on Verified locally on JDK 11: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java:62
NETTY_ADMIN_SIZEonly configures the number of admin event-loop threads (DefaultDriverOption.java:807-811); it does not configure anAddressResolverGroup. This link gives users an incorrect way to identify or change the resolver. Refer to a customNettyOptionsbootstrap hook instead, or omit the configuration link.
* <p><b>Note on resolver:</b> DNS lookup is performed via {@link
* InetAddress#getAllByName(String)} on the calling thread, bypassing any custom Netty {@code
* AddressResolverGroup} configured via {@link
* com.datastax.oss.driver.api.core.config.DefaultDriverOption#NETTY_ADMIN_SIZE}. This is
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java`:
- Line 603: Update the public Javadoc for the reconnection-plan option in
TypedDriverOption to state that it appends DNS-expanded candidates returned by
getResolvedContactPoints(), rather than raw original contact points, and that
monitors which re-resolve addresses skip this behavior; retain the documented
default of true.
In
`@core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java`:
- Around line 147-153: Prevent blocking DNS resolution from query-plan creation
by moving MetadataManager.getResolvedContactPoints() off the caller thread or
introducing bounded caching before using its results. Apply the fix to the
BEFORE_INIT/DURING_INIT path in
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java:147-153
and the control-reconnect path in
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java:164-184;
update core/src/main/resources/reference.conf:2321-2334 if needed so
fallback-to-original-contact-points is not enabled without bounded, non-blocking
resolution.
In `@core/src/main/resources/reference.conf`:
- Around line 2321-2334: The default for fallback-to-original-contact-points
must not enable the blocking DNS fallback path; change this configuration
default back to false while preserving the existing setting name and
documentation.
In
`@core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java`:
- Around line 512-529: The test should enforce expansion to the complete DNS
result set, not merely verify that one resolved node exists. Update
should_expand_unresolved_hostname_to_all_ips to obtain
InetAddress.getAllByName("localhost"), compare the returned node count and
endpoint addresses against all expected addresses on port 9042, and retain the
resolved-address assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 648940a1-36ee-47f0-8f02-aff008723307
📒 Files selected for processing (29)
core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.javacore/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.javacore/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.javacore/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.javacore/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.javacore/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.javacore/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.javacore/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/CloudTopologyMonitor.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/TopologyMonitor.javacore/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.javacore/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.javacore/src/main/resources/reference.confcore/src/test/java/com/datastax/dse/driver/internal/core/insights/InsightsClientTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/heartbeat/HeartbeatIT.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/resolver/MockResolverIT.java
🚧 Files skipped from review as they are similar to previous changes (11)
- core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java
- core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java
- core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
- core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java
- core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java
- core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java
- core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java
… (DRIVER-201) When RESOLVE_CONTACT_POINTS=false (the default) a hostname contact point was stored as a single unresolved InetSocketAddress, so the query plan tried only the first DNS IP. Keep contact points unresolved and expand each hostname to all its DNS IPs at query-plan time via MetadataManager.getResolvedContactPoints(), so the driver falls back to the next candidate when one IP is unreachable. Resolution is bounded, concurrent and best-effort. getResolvedContactPoints() runs on the admin event loop, where nothing should block, so each blocking InetAddress.getAllByName() call is offloaded to a cached daemon-thread pool and all unresolved hostnames are resolved concurrently against a single CONTACT_POINT_RESOLUTION_TIMEOUT deadline. A cached pool (rather than one shared thread) means each hostname resolves on its own thread, so one slow or blackholed lookup cannot starve the sibling contact points, nor the next reconnect that would otherwise queue behind it. If a hostname cannot be resolved or resolution times out, the original unresolved contact point is kept as-is rather than dropped, so the query plan is never emptier than the configured contact points and the address can still be resolved later at connection time (as it was before DNS expansion existed). This is an interim mitigation, superseded by scylladb#890's non-blocking EndPoint.resolveAll(). Default advanced.control-connection.reconnection.fallback-to-original-contact-points to true (no longer Experimental): it is the DNS re-resolution path on reconnect. Metadata nodes hold an already-resolved endpoint that is never re-resolved, so falling back to the original unresolved contact points re-expands the hostname to its current DNS IPs. Document that DNS-expanded contact points are IP-backed connection candidates that may be persisted in metadata, and that each synthetic endpoint retains the original hostname (built from the resolved InetAddress) so TLS peer host / SNI / hostname verification keep using the configured hostname. Gate the control-connection reconnection contact-point fallback behind a new TopologyMonitor.reresolvesNodeAddresses() (default false; true for the proxy-based ClientRoutesTopologyMonitor and CloudTopologyMonitor). Those monitors reach nodes through endpoints that already re-resolve on every connection attempt and maintain an authoritative node set, so appending raw contact points to their reconnection plan is unnecessary and could resurrect removed nodes (PrivateLink/Cloud regression safety). The reconnection plan also appends the contact points only once the load balancing policy is RUNNING, so the pre-init plan (already built from the resolved contact points) is not duplicated or re-resolved. Remove OptionalLocalDcHelper.checkLocalDatacenterCompatibility(): it warned when a contact point reported a different datacenter than the configured local DC. Since commit 12e6acb switched initial metadata refresh to hostId-only matching, contact-point nodes are never reused and their datacenter stays null; comparing a configured local DC against that null made the check fire as a false positive for every contact point whenever local-datacenter was set on the default profile, rather than surface a real mismatch. The node-based "configured local DC matches no node" warning (against discovered nodes whose datacenters are populated) is retained, so the only user-visible effect is that the spurious warning is no longer emitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 45 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
core/src/test/java/com/datastax/oss/driver/internal/core/metadata/TestNodeFactory.java:1
- Grammar in Javadoc: 'A endpoint' should be 'An endpoint'.
integration-tests/src/test/java/com/datastax/oss/driver/core/resolver/MockResolverIT.java:1 - This test mutates the global
ChannelFactorylogger level, which can cause cross-test interference if integration tests are executed in parallel (or if another test relies on the prior level). Consider avoiding global level changes by adding a DEBUG-level appender with an appropriate filter/threshold (or a dedicated test logger name) so capture is isolated to this test instance.
| // Concatenate rather than mutate: the RUNNING-state regularQueryPlan is a built-in QueryPlan | ||
| // whose add()/addAll() throw UnsupportedOperationException (poll() is its only mutator). | ||
| // CompositeQueryPlan drains the regular plan first, then the contact-point fallback. | ||
| return new CompositeQueryPlan(regularQueryPlan, new SimpleQueryPlan(contactNodes.toArray())); |
9b10855 to
8bae94c
Compare
|
Self-review round — no thread prompted these. Six fix-ups folded in with
The upgrade guide now says what that option bought, not only that it is inert: per-address Timeout note, in three places ( "Worst case is Nine new unit tests; |
37ffbb4 to
51009f0
Compare
dkropachev
left a comment
There was a problem hiding this comment.
Please add a cap on number of candidates it retries inside ChannelFactory.tryNextCandidate and randomly shuffle them every time.
| * doing this inside the handshake instead of after it: the alternative addresses are still | ||
| * available. | ||
| */ | ||
| private void onNodeInfo(Rows rows) { |
There was a problem hiding this comment.
Instead of that can you please create an interface for a async hook here, so that control connections could supply private method for it, channeling all the information it collected from the hook straight to control connection context
27e784f to
273060d
Compare
…VER-201) OptionalLocalDcHelper.checkLocalDatacenterCompatibility() warned when a contact point reported a datacenter different from the configured local DC. This has been dead code on scylla-4.x since 12e6acb: refresh matches nodes by hostId only, so contact-point nodes never get a datacenter assigned and the warning could never fire. Remove it. The separate "configured local DC matches no node" warning is retained. Nothing covered the removal, and CUSTOMER-588 is the bug the check caused: it compared the configured local DC against ephemeral placeholder Nodes (built by MetadataManager#addContactPoints via DefaultNode#newContactPoint, datacenter always null), so it warned unconditionally whenever a local DC was configured, no matter where the contact points actually were. The new test builds a placeholder Node the same way production does, plus a resolved node that genuinely is in the configured local DC, and asserts no warning is logged. It asserts on the absence of any WARN rather than of one particular message, so a regression under different wording is still caught; should_warn_if_configured_dc_matches_no_node is the positive control for the same appender, so a silent capture failure cannot make it pass by accident. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
273060d to
3811079
Compare
3811079 to
1e9bc64
Compare
| List<SocketAddress> shuffleAndLimit(List<? extends SocketAddress> addresses) { | ||
| List<SocketAddress> shuffled = new ArrayList<>(addresses); | ||
| if (shuffled.size() > 1) { | ||
| Collections.shuffle(shuffled, random); | ||
| } |
There was a problem hiding this comment.
This shuffles on every connect(), including connections to an already-identified node, and pin() binds only the channel's endpoint copy — never the Node's. So for a node whose EndPoint is an unresolved hostname with several A-records, each ChannelPool connection re-expands and re-shuffles independently, and different channels of the same pool can end up on different servers while the driver treats all of them as one node. Token-aware routing then sends replica requests to a non-replica, getOpenConnections() counts channels on other hosts, and per-node metrics merge two hosts.
| * for the cap that bounds what wrong credentials can cost. | ||
| */ | ||
| private static boolean isNodeWideFailure(Throwable error, boolean nodeIsIdentified) { | ||
| return error instanceof UnsupportedProtocolVersionException && nodeIsIdentified; |
There was a problem hiding this comment.
AuthenticationException is excluded here for every node, and the javadoc justifies it with "the addresses a name expands to may belong to different nodes". That premise is the one isIdentified() explicitly rules out for an identified node — its own javadoc says "every address of an identified node is that same node". So the two failure types are being judged on opposite premises.
Practical cost: with a genuinely wrong password, one pool reconnect now attempts up to max-candidate-addresses = 5 logins instead of 1, on every reconnection round. Against a server or IdP that throttles or locks accounts, that can lock out an application during a rollout.
| try { | ||
| boolean nodeWide = error != null && isNodeWideFailure(error, nodeIsIdentified); | ||
| if (error == null) { | ||
| resultFuture.complete(channel); |
There was a problem hiding this comment.
Return value dropped. Every sibling completion site in this file guards it deliberately — completeCandidate (:1331) and abandonCandidate (:1342) both forceClose() when they lose the race — and tryNextCandidate's own invariant comment only argues that exceptional double completion is harmless. If resultFuture is already completed (it is handed to callers as a CompletionStage), this drops a live socket and its pipeline for the life of the JVM.
| // | ||
| // The cost of leaving it out: a tagging generator can keep reporting under an endpoint string | ||
| // the node no longer answers to, until something else changes the prefix. | ||
| boolean differentMetricIdentity = |
There was a problem hiding this comment.
The commit message for this change says: "The test is therefore asMetricPrefix() plus toString(), because both are in use." The code tests asMetricPrefix() only, and the comment directly above spends a paragraph explaining why toString() is deliberately excluded ("asMetricPrefix() alone, deliberately…") and what that costs.
| .withDuration(DefaultDriverOption.CONNECTION_INIT_QUERY_TIMEOUT, Duration.ofSeconds(2)) | ||
| .withDuration(DefaultDriverOption.RECONNECTION_MAX_DELAY, Duration.ofSeconds(1)) | ||
| // These tests exercise heartbeat behavior only. Disable the contact-point | ||
| // reconnection fallback, which would otherwise send an extra OPTIONS message on |
There was a problem hiding this comment.
This comment says the fallback is disabled because it "would otherwise send an extra OPTIONS message on init/reconnect and skew the heartbeat counts". The commit body says it is because "an exhausted reconnection round retries roughly twice as many addresses". Those are unrelated mechanisms, so one of them is wrong. Could you reconcile them and keep whichever is the real reason?
…S (DRIVER-201) Contact points backed by a hostname are now always kept unresolved, so the connection layer can expand them to all their DNS-mapped IPs at connection time. SessionBuilder no longer reads RESOLVE_CONTACT_POINTS when merging contact points; the option is deprecated and has no effect. An already-resolved InetSocketAddress passed programmatically is still used as provided, with no further expansion. OptionsMap.fillWithDriverDefaults() still carries the option's reference.conf value so the defaults map stays complete, and is annotated accordingly -- the build treats deprecation warnings as errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Groundwork for expanding a hostname to all of its addresses: resolution becomes
the connection layer's job, so everything that produces an EndPoint stops doing
DNS of its own, and an endpoint gains a way to record which address a connection
actually reached.
PinnableEndPoint is the new internal contract: pinTo(SocketAddress) returns a
copy bound to one address, and the pin is excluded from equals(), hashCode(),
asMetricPrefix() and toString(). Endpoints are set and map keys, and node
metrics are named after them, so a pinned copy has to be indistinguishable from
its original everywhere except when the connection layer asks which address
answered. A generic delegating wrapper was rejected: its equals() would be
asymmetric, because DefaultEndPoint#equals tests instanceof and would reject the
wrapper while the wrapper accepted the original, and it would break
SniSslEngineFactory's instanceof SniEndPoint guard. Each implementation
therefore carries a nullable pinnedAddress of its own.
SniEndPoint additionally normalizes a resolved proxy *hostname* back to
unresolved. withCloudProxyAddress(new InetSocketAddress("proxy", 9042)) resolves
eagerly, which froze Cloud on whichever proxy IP the JVM happened to return; an
IP-literal proxy is left alone. Contact points keep the opposite policy on
purpose, since ContactPoints.merge() only ever applied its resolve flag to
config-file entries.
ClientRoutesTopologyMonitor.resolve() likewise returns the client route as an
unresolved address and no longer looks it up, which keeps it a pure in-memory
cache lookup that is safe to call from an event loop, and lets a custom resolver
apply to client routes just as it does to contact points. Its protected
resolveAddress() extension point, which existed only so tests could stub out
InetAddress.getByName, goes with it. This has to move together with
ClientRoutesEndPoint: dropping "throws UnknownHostException" from one and the
matching catch from the other is a single compilable change.
TopologyMonitor gains reresolvesNodeAddresses(), which tells the control
connection's reconnection query plan whether this monitor already keeps
addresses fresh. It defaults to false, correct for DefaultTopologyMonitor, whose
peers hold an already-resolved IP from system.peers. ClientRoutesTopologyMonitor
reports true only when every currently-known node actually has a live route:
where the route set is incomplete, ClientRoutesEndPoint falls back to a static
resolved endpoint, and those nodes still need the contact-point fallback.
The "is this a name" test several of these need is shared as
AddressUtils.carriesName(): a resolved address compares its host string against
the literal its own bytes produce, an unresolved one parses its host string.
Neither isUnresolved() nor the presence of an InetAddress can tell a name from a
literal on its own.
DseGssApiAuthProviderBase.serverName() falls back to getHostString() when
getAddress() returns null, which is now the ordinary case for a Cloud or
client-route endpoint rather than an impossible one.
EndPoint.resolve() keeps its signature and is not deprecated, so third-party
implementations still compile. Its javadoc gains two expectations: return the
address as-is rather than looking names up, since this is now called from an
event loop; and callers are warned that the returned address is no longer always
resolved, so getHostString() is the safe way to read the host.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…R-201) This is the fix for DRIVER-201. When a contact point or a node address is a hostname that maps to several IPs, the driver used to try only the first one and raise AllNodesFailedException if it was unreachable, even though the hostname also resolved to healthy addresses. Resolution is a connection-layer concern. ChannelFactory.connect() is now the single place that turns "the address this node is known by" into "the addresses to actually try": EndPoint.resolve() yields one address and does no lookup, so it stays safe to call from an event loop; ChannelFactory expands it through the bootstrap's Netty AddressResolverGroup; the candidates are tried in sequence until one connects; and the endpoint is pinned to the address that won, so the channel carries the address it is really on. Expansion always goes through the configured resolver, mirroring Netty's own doResolveAndConnect0 short-circuit (no group, !isSupported, isResolved) rather than pre-filtering on isUnresolved(). Both isSupported() and isResolved() are overridable, so a redirecting custom resolver keeps its say over addresses that merely look resolved. The bootstrap is built once per connect() and cloned per attempt, with the clone's resolver disabled: Bootstrap.clone() carries the resolver over, so an enabled clone would resolve each candidate a second time -- through resolve(), singular -- and a redirecting resolver would collapse every candidate onto its first answer, silently killing the fallback. Details that took a round each to get right: - The queried hostname is re-attached to resolver-returned addresses, centrally rather than per endpoint, so TLS sees the name the user configured instead of an IP or a CNAME label. Scoped IPv6 keeps its zone via the numeric Inet6Address.getByAddress overload; the NetworkInterface one re-derives the scope and throws when the interface has no address of the same local type. - One EventLoop is chosen per connect() and shared by resolution and every clone(eventLoop), instead of letting Bootstrap.connect() advance the chooser a second time and land channels on half the loops. - Candidates are shuffled per connect and truncated to the new advanced.connection.max-candidate-addresses option (default 5). The shuffle spreads load and varies the starting address between attempts with no per-name state to maintain; the cap bounds what one attempt can cost -- each address tried is a full connect plus handshake, and with wrong credentials a rejected login -- while successive attempts sample fresh random subsets, so no address is permanently out of reach. - Protocol-version rejection is terminal only for a node whose host id is known. The addresses of an unidentified endpoint may belong to different nodes, and collapsing a contact-point hostname into one Node must not lose the query-plan advance that resolve-contact-points=true used to provide. An authentication failure is never terminal, even for an identified node: with a multi-record name it may be specific to the address (a stale record pointing at a foreign cluster fails at AUTH, which runs before the cluster-name check), a single address's failure must not write off the endpoint, and the candidate cap is what bounds the cost of genuinely wrong credentials. - Failures from earlier candidates are attached to the final error as suppressed exceptions -- with the surfaced one chosen by how callers classify errors rather than by position -- and negotiation history is scoped per candidate address. - Every resolver and Netty callback completes the connect future on failure. connect() has no timeout at the resolution stage, so an unguarded throw would hang the caller for good. afterBootstrapInitialized() now runs once per logical connection rather than once per attempt, and sees the bootstrap before the driver's handler is installed; a handler set by the hook is overwritten, with a one-time warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…IVER-201) Node metrics are named after the endpoint, so DefaultNode.setEndPoint() has to re-register them whenever those names change -- which is not the same question as whether this is a different node, and the old !equals() test got it wrong in both directions. It was too narrow: an unresolved hostname and the resolved address it maps to compare *equal* while their metric prefixes differ, which is exactly what happens when a contact-point node adopts the endpoint built from its system.local row. And too wide in the other direction is now possible too, since a pinned copy differs from its original only by an address that both equals() and the metric identity ignore by contract. The test is therefore asMetricPrefix() plus toString(), because both are in use: the default MetricIdGenerator names node metrics after the prefix, the tagging one tags them with toString(). The pin is excluded from toString() as well, or DefaultTopologyMonitor#buildNodeEndPoint returning the control channel's pinned endpoint for the system.local row would silently retag node metrics on every refresh and orphan the old series. The node also adopts the newest endpoint instance even when it compares equal, since a pinned copy carries the address every subsequent connection will use. Finally, the rebuild order is clear, then swap, then build. Dropwizard and MicroProfile do not remember the ids they registered under; their clearMetrics() recomputes each one from the node's endpoint as it stands at that moment. The previous order -- swap, build, clear -- therefore deleted exactly the series the new updater had just registered and left the old ones behind with nothing writing to them. That ordering is upstream's, but it used to be reached only when the endpoints compared unequal; keying the rebuild on metric identity brings the ordinary contact-point transition onto the same path. The pre-existing pin test was vacuous: a mocked context yields NoopNodeMetricUpdater, for which the rebuild is skipped entirely. Both tests now stub MetricsFactory, and the ordering test drives a real MetricRegistry through a hostname-to-IP rename; it was proven to fail under the old order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e loop (DRIVER-201) ChannelFactory tries every address a contact-point hostname resolves to while it opens a channel, but the node's identity was read afterwards, over the channel that won -- by then the remaining candidates are gone. ControlConnection advanced its query plan on that failure, and since a contact-point hostname is now a single Node, that wrote off the whole hostname on the strength of one of its addresses. With a single contact point and the default reconnect-on-init=false, session initialization failed outright, and a rebuilt session failed the same way every time while a healthy address sat unused. The identity read now happens while the factory still holds the remaining candidates, through a caller-supplied hook. DriverChannelOptions gains an internal ConnectHook (precedent for a behavioral member there: eventCallback) plus a timeout; after protocol initialization succeeds on a candidate, ChannelFactory invokes the hook and treats a rejection, a synchronous throw or a timeout as a per-candidate failure: the channel is closed and the loop advances to the endpoint's next address, exactly like an init failure. ControlConnection arms the hook only for a node whose host id is unknown -- contact points, the one case with something to learn. The hook runs TopologyMonitor.getChannelNodeInfo, so a custom monitor's identity read is honored; it rejects a node that reports no host id, and channels what it read straight into a per-attempt holder. Once the connect completes, the captured NodeInfo is registered without a second read, after asserting it came from the winning channel; a miss (a ChannelFactory subclass that skips the hook) falls back to a direct read. The options are built fresh per attempt, because the holder is stateful and overlapping connect chains are reachable: the initial connect() runs outside Reconnection, and reconnectNow() checks only initWasCalled. Pool connections and reconnects to identified nodes carry no hook and send exactly the bytes they sent before. REGISTER moves out of the init handshake to keep its ordering property: identity is validated before the channel registers for events. ChannelFactory sends it through AdminRequestHandler after the hook accepts, with the same init-query timeout; a registration failure is a per-candidate failure, as it was as an init step, and the CLIENT_ROUTES_CHANGE rejection keeps its clear message. The one visible cost: the window in which a live channel is not yet registered for events grows by the hook's round trip. The wire cost is unchanged from reading identity after the connect: one "SELECT * FROM system.local WHERE key='local'" per contact-point connection, now inside the attempt instead of after it. Init itself ends at the cluster-name check (or SET_KEYSPACE), byte-identical to the pre-multi-address exchange, which is what ProtocolVersionMixedClusterIT pins. Two consequences are visible. A contact point that exhausts every address on identity failures now fires controlConnectionFailed and is marked DOWN pre-init, the treatment connect-phase failures already get; and AllNodesFailedException reports one entry per contact point, with the per-address failures attached as suppressed exceptions. One pre-existing exposure stays pre-existing rather than closing: ChannelFactory imprints the cluster name, product type and negotiated protocol version on init success, so a candidate the hook then rejects has already imprinted -- like every other channel abandoned after init (a node turned IGNORED, a close during resolve). The values are properties of the cluster that answered on that address, so this is identical to the behavior before this series. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n (DRIVER-201) advanced.control-connection.reconnection.fallback-to-original-contact-points now defaults to true, and is the driver's DNS re-resolution path. Nothing else re-resolves. Metadata nodes hold an endpoint built from an already-resolved system.peers IP, and the control node's own endpoint is pinned by ChannelFactory to the single address its connection reached, deliberately, so that a node with a known identity cannot wander to a different host. Once the records behind a hostname change, appending the original contact points is therefore the only way back: they are still unresolved hostnames, so ChannelFactory expands each one to its current IPs at connection time. The append is gated on the topology monitor not re-resolving addresses itself, since a proxy-based monitor keeps them fresh and raw contact points could resurrect nodes it has authoritatively removed. The exception is an empty regular plan: with no live node to try, reconnection cannot recover on its own. The plans are concatenated rather than mutated. A RUNNING-state query plan is a built-in QueryPlan whose add()/addAll() throw UnsupportedOperationException, poll() being its only mutator, so with the fallback defaulting on every post-init control reconnect would otherwise have crashed. The append is also skipped before the LBP reaches RUNNING, where newQueryPlan() has already built the plan from the contact points and appending would duplicate every entry. Documented cost: the contact points are appended without being compared against the live-node plan, because at plan time they are hostnames while the live nodes are resolved IPs. When DNS has not changed they expand to addresses the plan just failed on, so an exhausted reconnection round retries roughly twice as many addresses -- which is why HeartbeatIT has to disable it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rewrites the address-resolution manual page around the connection layer doing the expansion, and adds an upgrade-guide section covering what changes for users: - there is no public API change, but EndPoint.resolve() may now return an unresolved address for Cloud/SNI and client-route nodes, so a caller doing ((InetSocketAddress) resolve()).getAddress().getHostAddress() gets a NPE where it previously worked; getHostString() is the safe read; - advanced.resolve-contact-points is deprecated and inert; - fallback-to-original-contact-points defaults to true, with its cost stated; - a contact point that none of its addresses can identify is now marked down before initialization completes, firing an event it did not fire before; - AllNodesFailedException reports one entry per contact point, with each address's failure attached as a suppressed exception; - the one-time TaggingMetricIdGenerator node-tag rename for hand-built Cloud proxy addresses; - the afterBootstrapInitialized() contract change; - two protected methods removed from internal classes that a subclass could have overridden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MockResolverIT drives the end-to-end fix through a JVM-level InetAddress hook: a hostname that maps to one dead and one live address must still produce a working session. Its multi-address test was one change away from being vacuous. The comment claimed the dead record was tried first because of resolver insertion order, but rotate() sorts candidates by toString() and discards that order; the dead address went first only because the sort is lexicographic. The test now captures ChannelFactory at DEBUG and requires the "trying next address" event, which was proven load-bearing: moving the dead IP to one that sorts last makes it fail in 7s instead of passing in 89s. ClientRoutesIT asserts on host strings rather than resolved IPs, since a client route now stays unresolved until the connection layer expands it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1e9bc64 to
249665f
Compare
Problem
DRIVER-201: when a contact point or a cluster node is given as a hostname that maps to several IPs (a DNS round-robin or dynamic-DNS entry), the driver only ever tried the first address — at initial contact, at connection time, and on control-connection reconnect. If that IP was unreachable, the driver raised
AllNodesFailedExceptionwhile the same hostname also resolved to healthy IPs.This PR expands every such hostname to all of its addresses and tries each in turn, for every connection the driver opens.
Design
Name resolution is a connection-layer concern.
ChannelFactory.connect()is the single place that turns "the address this node is known by" into "the addresses to actually try":EndPoint.resolve()yields one address and performs no lookup, so it stays safe to call from an event loop.ChannelFactoryexpands it through the bootstrap's NettyAddressResolverGroup— the resolver an unresolved address already reached viaBootstrap.connect()— so a custom resolver fromNettyOptions.afterBootstrapInitialized()keeps applying,disableResolver()is honoured, and whether an address needs resolving stays the resolver's call.No public API change.
EndPoint.resolve()keeps its signature and is not deprecated. Its javadoc gains one expectation: return the address as-is rather than looking names up. There is one behaviour change for callers ofnode.getEndPoint().resolve():resolve()returnssystem.peers, and the node the control connection is onFor the last two
getAddress()now returnsnull; usegetHostString(), which covers both forms and never triggers a reverse lookup.The candidate loop
ChannelPool#handleErrormaps a cluster-name mismatch and a protocol-version rejection toTopologyEvent.forceDown) surface only when every candidate failed that way: one stale record fronting another cluster must not write off a node whose other addresses merely timed out.advanced.connection.max-candidate-addressesare tried per attempt (new option, default 5); addresses beyond the cap are not lost, as each attempt samples afresh.UnsupportedProtocolVersionExceptionagainst a node whose host id is known. An authentication failure is never terminal — with a multi-record name it may be specific to the address (a stale record pointing at a foreign cluster fails at AUTH, which runs before the cluster-name check), and the cap bounds the cost of wrong credentials.Pinning:
PinnableEndPointA name describes a set of addresses; a channel is connected to one. Pinning gives node identity (a node that answered as host id X keeps reconnecting to that IP) and keeps the channel path lookup-free — SSL engine creation, GSSAPI service-name lookup and
DefaultTopologyMonitor#savePortall callresolve()on the channel's endpoint, which on a pinned copy is a field read. A pinned copy is otherwise indistinguishable from the original (sameequals,hashCode,asMetricPrefix(),toString()), because nodes adopt pinned copies.PinnableEndPointis internal.Accepting a candidate: the connect hook
The control node's identity used to be read after the channel was chosen, so a failure there wrote off a whole hostname on the strength of one address: with a single contact point and the default
advanced.reconnect-on-init = false, initialization failed outright while a healthy address sat unused.The read now happens while the factory still holds the remaining candidates, through an internal async
ConnectHookonDriverChannelOptions. A rejection, synchronous throw or timeout is a per-candidate failure: the channel is closed and the loop advances, exactly like an init failure.ControlConnectionsupplies the hook only for a node whose host id is unknown, i.e. contact points; it runsTopologyMonitor.getChannelNodeInfo, rejects a node reporting nohost_id, and hands what it read toregisterNodewithout a second read. Pool connections and reconnects to identified nodes carry no hook and send exactly the bytes they sent before.REGISTER moved out of the init handshake so that a channel the hook is about to reject never registers for events; it is sent after the hook accepts, under the same init-query timeout, and fails per candidate as it did as an init step. Init itself still ends at the cluster-name check (or
SET_KEYSPACE) — byte-identical to the pre-multi-address exchange, which is whatProtocolVersionMixedClusterITpins. Since init is no longer the last word on a candidate, the negotiated state (protocol version, cluster name, product type) is latched only once a candidate is accepted: a rejected one must not leave a foreign cluster's name behind for every later connection to trip over.Changes
advanced.resolve-contact-pointsis deprecated and has no effect (it only ever applied to config contact points; a programmatic already-resolvedInetSocketAddressis still used as provided).DefaultEndPointreturns its address as-is and implementsPinnableEndPoint.SniEndPointandClientRoutesEndPointhand the proxy/route address over unresolved instead of resolving it themselves, so all A-records are tried within one attempt and a custom Netty resolver applies to those paths for the first time.CompositeQueryPlan(regularPlan, new SimpleQueryPlan(contactNodes))instead of mutating the policy's plan — built-inQueryPlans rejectaddAll(), which threw on every post-init reconnect once the fallback defaulted on.fallback-to-original-contact-pointsnow defaults totrue: it is the driver's DNS re-resolution path.NettyOptions.afterBootstrapInitializedcontract documented: the driver installs its own handler afterwards (one set by the hook is replaced, now warned about once), and the resolver configured there is what expands names.OptionalLocalDcHelper: removed the deadcheckLocalDatacenterCompatibility()check — contact-point nodes never get a datacenter, so it compared againstnulland could only fire spuriously. Called out because it touches aprotectedextension point.Operator-visible consequences are in
upgrade_guide/README.md: the deprecated option, theresolve()change above, the control node's metric prefix moving to its own address, one-entry-per-contact-pointAllNodesFailedException, a contact point now being marked DOWN when no address can identify it, the shuffle and the new cap, and a one-timeTaggingMetricIdGeneratornode-tag rename for hand-built Cloud proxy addresses.Tests
Unit coverage sits with the mechanism it exercises:
ChannelFactoryNettyResolverTest(custom resolvers,disableResolver(), pass-through and redirect, still-unresolved expansions),ChannelFactoryMultiAddressTest(fallback with suppressed causes, error classification, shuffle vs. resolver order, the cap, hostname re-attachment),ChannelFactoryConnectHookTest(hook ordering, rejection/throw/timeout, a zero timeout meaning unbounded, REGISTER placement, the negotiated-state latch),ControlConnectionTest(hook arming, capturedNodeInfo, the channel/node-info pairing under concurrency, exclusion after handshake),DefaultTopologyMonitorTest, the three*EndPointTests,DefaultNodeTest,DropwizardNodeMetricUpdaterTest,LoadBalancingPolicyWrapperTest, plusChannelFactoryPinnedEndPointTest/ProtocolNegotiationTestandAddressUtilsTest.ProtocolInitHandlerTestpins that init sends noRegisterframe even when events are requested, andProtocolVersionMixedClusterITis unmodified — it pins the exact init sequence, so it is the proof that no bytes changed.MockResolverITcovers the end-to-end path against a live cluster through a JVM-level DNS hook, including a multi-record name carrying a dead record.Verified on JDK 11 at the current head: full
coreunit suite (4024 tests), the dependent modules (metrics/*,query-builder,mapper-runtime),fmt:check, and a per-commit compile.MockResolverITrides CI.